Skip to content

Fix #12910 FP containerOutOfBounds with overloaded template function#7453

Merged
chrchr-github merged 3 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_12910
Apr 16, 2025
Merged

Fix #12910 FP containerOutOfBounds with overloaded template function#7453
chrchr-github merged 3 commits intocppcheck-opensource:mainfrom
chrchr-github:chr_12910

Conversation

@chrchr-github
Copy link
Copy Markdown
Collaborator

Feels hacky though.

Comment thread lib/symboldatabase.cpp Outdated
if (matches.size() == 1)
if (matches.size() == 1 && std::none_of(functionList.begin(), functionList.end(), [tok](const Function& f) {
const auto nTok = tok->str().size();
return startsWith(f.name(), tok->str()) && f.name().size() > nTok + 2 && f.name()[nTok + 1] == '<';
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return startsWith(f.name(), tok->str()) && f.name().size() > nTok + 2 && f.name()[nTok + 1] == '<';
return startsWith(f.name(), tok->str() + "<") && f.name().size() > nTok + 2;

I still feel it's hacky but this seems better.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function name seems to be func < arg > instead of func<arg> for some reason.

Copy link
Copy Markdown
Collaborator

@danmar danmar Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah there is (sometimes?) space before the "<" . we did not used to have it. maybe the extra spaces are removed in the future again who knows.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have examples for either case I will see if I can bisect this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have examples for either case I will see if I can bisect this.

The spaces appear in the output of --debug -v (e.g. name:), but I don't know what the significance is.

Copy link
Copy Markdown
Collaborator

@firewave firewave Apr 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

template <class T> void fun()
{
}

int main()
{
    fun<int>();
}

1.77

1:
|
4:
5: int main ( )
6: {
7: fun<int> ( ) ;
8: } void fun<int> ( )
2: {
3: }
[...]
Scope: 000001F7E6853E30 Function
    className: fun<int>
[...]
##AST
( 'signed int'
`-main

( 'void'
`-fun<int>

( 'void'
`-fun<int>

1.78

1:
|
4:
5: int main ( )
6: {
7: fun < int > ( ) ;
8: } void fun < int > ( )
2: {
3: }
[...]
Scope: 000001CBB41650D0 Function
    className: fun < int >
[...]
##AST
( 'signed int'
`-main

( 'void'
`-fun < int >

( 'void'
`-fun < int >

1.82

1:
|
4:
5: int main ( )
6: {
7: fun<int> ( ) ;
8: } void fun<int> ( )
2: {
3: }
[...]
Scope: 000001E34CC268D0 Function
    className: fun < int >
[...]
##AST
( 'signed int'
`-main

( 'void'
`-fun < int >

( 'void'
`-fun < int >

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 1.78 change has been bisected to 0943b21.

The 1.82 change has been bisected to eaadfb3.

If we make any changes we should add unit and Python tests for this.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something in here we need to file a ticket about?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inconsistent use of spaces in template names, perhaps.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danmar
Copy link
Copy Markdown
Collaborator

danmar commented Apr 13, 2025

Feels hacky though.

I agree.. it feels like there could be cases that does not work as wanted.

Copy link
Copy Markdown
Collaborator

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well I can accept this. If the space is removed in the future we'll have to change this.

@chrchr-github chrchr-github merged commit f837929 into cppcheck-opensource:main Apr 16, 2025
53 checks passed
@chrchr-github chrchr-github deleted the chr_12910 branch April 16, 2025 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants